-
Notifications
You must be signed in to change notification settings - Fork 1
[Refactor] SLO에 영향을 미치는 쿼리 개선 작업 #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- EntityGraph에서 Batchsize 조절로 쿼리 로직 변경
- EntityGraph에서 Batchsize 조절로 쿼리 로직 변경
- EntityGraph에서 Batchsize 조절로 쿼리 로직 변경
Walkthrough페이지네이션 도입과 연관 최적화 주석 추가가 중심이다. 리포지토리/서비스에서 List 반환을 Page로 전환하고 일부 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant StoreService
participant StoreRepository
participant DB
Client->>StoreService: getStores(..., page,size,sort)
StoreService->>StoreRepository: findAllByConditions(..., PageRequest)
StoreRepository->>DB: SELECT ... (Specification, JOIN tags, DISTINCT when set)
DB-->>StoreRepository: Page<Store> (content + metadata)
StoreRepository-->>StoreService: Page<Store>
StoreService-->>Client: StoresResponse (from page.content)
sequenceDiagram
autonumber
participant Client
participant CheerService
participant CheerRepository
participant DB
Client->>CheerService: getCheersByStoreId(storeId, page,size)
CheerService->>CheerRepository: findAllByStoreOrderByCreatedAtDesc(store, PageRequest)
CheerRepository->>DB: SELECT ... WHERE store_id=? ORDER BY created_at DESC
DB-->>CheerRepository: Page<Cheer>
CheerRepository-->>CheerService: Page<Cheer>
CheerService-->>Client: List<CheerResponse> (from page.content)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/eatda/repository/store/StoreRepository.java (1)
1-66: @batchsize 어노테이션 누락
엔티티 클래스(Store.cheers등)의 컬렉션 필드에@BatchSize(size = 30)어노테이션을 적용하세요.spring.jpa.properties.hibernate.default_batch_fetch_size=30설정은 이미 확인되었습니다.
🧹 Nitpick comments (2)
src/main/java/eatda/domain/store/Store.java (1)
60-65: 주석 내용을 현재 설정과 일치하도록 조정해주세요.
application-*.yml에서spring.jpa.properties.hibernate.default_batch_fetch_size가 30으로 설정되어 있는데, 여기서는 BatchSize=10이라고 설명하고 있어 혼선을 줄 수 있습니다. 실제 동작에 맞춰 주석을 업데이트하거나, Collection 레벨에서 10을 강제하려면 정확한 설정 방법(@batchsize 등)을 주석에 함께 명시해주세요.src/main/java/eatda/repository/cheer/CheerRepository.java (1)
1-58: Cheer 엔티티 연관관계에 @batchsize 누락
member, store, images 필드에 @batchsize 설정이 없습니다. cheerTags는 Embedded라 현 구조에서 적용이 불가능하므로, 추후 Embedded 제거 후 OneToMany 매핑 전환을 통해 @batchsize를 추가 적용하는 방안을 검토해주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/eatda/domain/cheer/Cheer.java(1 hunks)src/main/java/eatda/domain/store/Store.java(1 hunks)src/main/java/eatda/repository/cheer/CheerRepository.java(3 hunks)src/main/java/eatda/repository/store/StoreRepository.java(3 hunks)src/main/java/eatda/repository/story/StoryRepository.java(0 hunks)src/main/java/eatda/service/cheer/CheerService.java(3 hunks)src/main/java/eatda/service/store/StoreService.java(2 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)src/main/resources/application-prod.yml(1 hunks)src/test/java/eatda/repository/cheer/CheerRepositoryTest.java(6 hunks)src/test/java/eatda/repository/store/StoreRepositoryTest.java(6 hunks)terraform/common/waf/main.tf(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/eatda/repository/story/StoryRepository.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
src/main/java/eatda/repository/store/StoreRepository.java (3)
11-11: 페이지네이션 지원을 위한 임포트 추가 확인
Page임포트 추가가 적절합니다. 이는List<Store>에서Page<Store>로의 반환 타입 변경을 지원합니다.
54-59: 쿼리 null 체크 추가 확인
query.distinct(true)호출 전 null 체크를 추가한 것은 좋은 방어적 프로그래밍입니다. JPA Specification에서 count 쿼리 실행 시 query가 null일 수 있으므로, 이 체크는NullPointerException을 방지합니다.
36-44: 페이지네이션 반환 타입 변경 확인됨
findAllByConditions가 서비스 및 테스트에서 모두Page<Store>로 사용되고 있어, 의도한 대로 반환 타입 변경이 정상 적용되었습니다.src/main/java/eatda/repository/cheer/CheerRepository.java (4)
11-11: 페이지네이션 지원을 위한 임포트 추가 확인
Page임포트 추가가 적절합니다. 이는 반환 타입을List<Cheer>에서Page<Cheer>로 변경하는 것을 지원합니다.
38-43: 쿼리 null 체크 추가 확인
query.distinct(true)호출 전 null 체크를 추가한 것은StoreRepository와 동일한 패턴으로 일관성 있게 적용되었습니다. 이는 JPA count 쿼리 실행 시 발생할 수 있는 NPE를 방지합니다.
51-51: findAll 메서드 시그니처 변경 확인
findAll(Specification<Cheer>, Pageable)메서드의 반환 타입을Page<Cheer>로 변경한 것이 적절합니다.
20-27: 페이지네이션 메서드 호출부 처리 확인 완료
findAllByStoreOrderByCreatedAtDesc와findAllByConditions호출부가 서비스와 테스트에서 모두Page<Cheer>를 적절히 처리하고 있어 추가 변경이 필요 없습니다.
leegwichan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
승로님 고생하셨습니다! 질문 1개 남겼으니 답변 부탁드릴께요!
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코멘트 남겼습니다 😄
다만.. 어떤 부분을 중점적으로 답변드려야 할지 조금 어려웠는데요.
앞으로는 걱정되는 지점을 조금 더 구체적으로 적어주시면 감사하겠습니다.
예를 들어 null 체크가 불필요하다고 보신 건지, distinct 로직이 궁금하신건지 등..
명확히 말씀해주시면 저도 더 빠르고 정확하게 답변드릴 수 있을 것 같아요 😅
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/eatda/domain/store/Store.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
|
🎉 This PR is included in version 1.8.0-develop.17 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.9.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



✨ 개요
SLO에 영향을 미치는 쿼리 성능 개선
🧩 문제 원인
EntityGraph+Pageable조합으로 인해 인메모리 페이징이 발생했습니다./api/shop,/api/store,/api/cheerAPI의 응답 지연으로 이어졌습니다.🛠️ 해결 방법
EntityGraph를 유지하면서 DB 레벨에서 페이징을 적용하려면쿼리 분리(Query Split) 또는 커스텀 QueryDSL 작성이 필요합니다.
BatchSize를 활용하여Lazy 로딩 시 발생하는 N+1 문제를 완화하고 SLO 영향을 최소화하는 방향으로 접근했습니다.
📍 적용 범위
/api/shop,/api/store,/api/cheerAPI의EntityGraph를 제거하고Batch Fetch를 적용했습니다.→ 최적 배치 크기는 10이지만, 향후 데이터 증가를 고려해 30으로 설정했습니다.
WAF 브라우저 요청 허용 규칙 개선
🧾 관련 이슈
#205 #207
🔍 참고 사항
Summary by CodeRabbit